Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More consistent handling of boss keys from key rings #2149

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

fenhl
Copy link
Collaborator

@fenhl fenhl commented Dec 10, 2023

Fixes #2137. In small keysy with keyrings give boss keys, boss doors of dungeons with keyrings are now unlocked. Tested with this plando:

{
    "settings": {
        "open_forest": "open",
        "starting_age": "adult",
        "spawn_positions": ["adult"],
        "starting_items": {
            "Hover Boots": 1
        },
        "shuffle_smallkeys": "remove",
        "key_rings_choice": "choice",
        "key_rings": ["Fire Temple"],
        "keyring_give_bk": true,
        "starting_hearts": 20
    },
    "entrances": {
        "Adult Spawn -> Temple of Time": {
            "region": "DMC Lower Local",
            "from": "GC Darunias Chamber"
        }
    }
}

This PR also introduces the methods World.keyring and World.keyring_give_bk to consolidate the checks for the various conditions required for a dungeon to have a key ring, or to have the boss key included in the key ring, respectively.

This also reverts #2045 which was intended to fix this issue but does not actually seem to do anything as far as I can tell. The approach taken there was to give the boss keys as starting items rather than unlocking the boss doors, with the reason given that having some boss doors but not others be unlocked would be misleading in RSL. Personally I think this PR's consistent behavior is a better approach overall as the locked boss door may lead players who are familiar with how keysy normally works to think they're stuck; RSL players would just have one more thing to keep in mind for identifying settings. Alternatively, we could indicate boss keysy by unlocking the door and giving the player the boss key item — keeping a boss key after using it is the vanilla behavior after all.

@fenhl fenhl added Type: Bug Something isn't working Component: Plandomizer Plandomizer library and functionality Component: Patching Affects the patching of the ROM labels Dec 12, 2023
@fenhl fenhl added this to the 8.1 milestone Feb 4, 2024
@fenhl fenhl added Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested labels Feb 4, 2024
Copy link
Collaborator

@cjohnson57 cjohnson57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to think about it but these changes make sense to me. Code is definitely cleaner. Especially the changes in get_doors_to_unlock

@cjohnson57 cjohnson57 removed Status: Needs Review Someone should be looking at it Status: Needs Testing Probably should be tested labels Feb 10, 2024
@cjohnson57 cjohnson57 merged commit 0254b16 into OoTRandomizer:Dev Feb 10, 2024
3 checks passed
@fenhl fenhl deleted the boss-key-ring-refactor branch February 10, 2024 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Patching Affects the patching of the ROM Component: Plandomizer Plandomizer library and functionality Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removing Small Keys while key rings with boss keys are enabled removes boss keys from seed
2 participants